From 2b46d039c01b56e895adf012ed34103e06e18835 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 3 Sep 2014 11:52:47 -0700 Subject: [PATCH] Implement `features` This feature was outlined in #385 [1], and documentation has been included as part of this commit. [1]: https://github.com/rust-lang/cargo/issues/385#issuecomment-53917539 --- Makefile.in | 2 +- src/bin/bench.rs | 6 +- src/bin/build.rs | 6 +- src/bin/doc.rs | 7 +- src/bin/run.rs | 6 +- src/bin/test.rs | 6 +- src/cargo/core/dependency.rs | 43 ++- src/cargo/core/manifest.rs | 14 +- src/cargo/core/resolver.rs | 272 +++++++++++---- src/cargo/core/summary.rs | 63 +++- src/cargo/ops/cargo_compile.rs | 15 +- src/cargo/ops/cargo_fetch.rs | 9 +- src/cargo/ops/cargo_generate_lockfile.rs | 8 +- src/cargo/ops/cargo_rustc/fingerprint.rs | 11 +- src/cargo/ops/cargo_rustc/mod.rs | 15 +- src/cargo/ops/cargo_upload.rs | 13 +- src/cargo/sources/registry.rs | 81 +++-- src/cargo/util/toml.rs | 97 +++--- src/doc/manifest.md | 131 +++++++ tests/test_cargo_compile.rs | 4 +- tests/test_cargo_features.rs | 426 +++++++++++++++++++++++ tests/test_cargo_registry.rs | 4 +- tests/tests.rs | 1 + 23 files changed, 1039 insertions(+), 201 deletions(-) create mode 100644 tests/test_cargo_features.rs diff --git a/Makefile.in b/Makefile.in index 066d28275..df0d873a3 100644 --- a/Makefile.in +++ b/Makefile.in @@ -83,7 +83,7 @@ $(TARGET_ROOT)/snapshot/bin/cargo$(X): src/snapshots.txt # === Tests -test: test-unit style no-exes $(foreach target,$(CFG_TARGET),test-unit-$(target)) +test: style no-exes $(foreach target,$(CFG_TARGET),test-unit-$(target)) style: sh tests/check-style.sh diff --git a/src/bin/bench.rs b/src/bin/bench.rs index 20a856eb5..d57f11495 100644 --- a/src/bin/bench.rs +++ b/src/bin/bench.rs @@ -16,6 +16,8 @@ Options: -h, --help Print this message --no-run Compile, but don't run benchmarks -j N, --jobs N The number of jobs to run in parallel + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple --manifest-path PATH Path to the manifest to build benchmarks for -v, --verbose Use verbose output @@ -24,7 +26,7 @@ All of the trailing arguments are passed to the benchmark binaries generated for filtering benchmarks and generally providing options configuring how they run. ", flag_jobs: Option, flag_target: Option, - flag_manifest_path: Option) + flag_manifest_path: Option, flag_features: Vec) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path)); @@ -39,6 +41,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult jobs: options.flag_jobs, target: options.flag_target.as_ref().map(|s| s.as_slice()), dev_deps: true, + features: options.flag_features.as_slice(), + no_default_features: options.flag_no_default_features, }, }; diff --git a/src/bin/build.rs b/src/bin/build.rs index f8a57d82a..eaa433a25 100644 --- a/src/bin/build.rs +++ b/src/bin/build.rs @@ -17,12 +17,14 @@ Options: -h, --help Print this message -j N, --jobs N The number of jobs to run in parallel --release Build artifacts in release mode, with optimizations + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple -u, --update-remotes Deprecated option, use `cargo update` instead --manifest-path PATH Path to the manifest to compile -v, --verbose Use verbose output ", flag_jobs: Option, flag_target: Option, - flag_manifest_path: Option) + flag_manifest_path: Option, flag_features: Vec) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { debug!("executing; cmd=cargo-build; args={}", os::args()); @@ -43,6 +45,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult jobs: options.flag_jobs, target: options.flag_target.as_ref().map(|t| t.as_slice()), dev_deps: false, + features: options.flag_features.as_slice(), + no_default_features: options.flag_no_default_features, }; ops::compile(&root, &mut opts).map(|_| None).map_err(|err| { diff --git a/src/bin/doc.rs b/src/bin/doc.rs index 8e4f7e21d..c24ae47ac 100644 --- a/src/bin/doc.rs +++ b/src/bin/doc.rs @@ -15,6 +15,8 @@ Options: -h, --help Print this message --no-deps Don't build documentation for dependencies -j N, --jobs N The number of jobs to run in parallel + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature -u, --update-remotes Deprecated option, use `cargo update` instead --manifest-path PATH Path to the manifest to document -v, --verbose Use verbose output @@ -22,7 +24,8 @@ Options: By default the documentation for the local package and all dependencies is built. The output is all placed in `target/doc` in rustdoc's usual format. ", flag_jobs: Option, - flag_manifest_path: Option) + flag_manifest_path: Option, + flag_features: Vec) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { shell.set_verbose(options.flag_verbose); @@ -38,6 +41,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult jobs: options.flag_jobs, target: None, dev_deps: false, + features: options.flag_features.as_slice(), + no_default_features: options.flag_no_default_features, }, }; diff --git a/src/bin/run.rs b/src/bin/run.rs index 4e0ed4d83..82b8eb49b 100644 --- a/src/bin/run.rs +++ b/src/bin/run.rs @@ -16,6 +16,8 @@ Options: -h, --help Print this message -j N, --jobs N The number of jobs to run in parallel --release Build artifacts in release mode, with optimizations + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple -u, --update-remotes Deprecated option, use `cargo update` instead --manifest-path PATH Path to the manifest to execute @@ -23,7 +25,7 @@ Options: All of the trailing arguments are passed as to the binary to run. ", flag_jobs: Option, flag_target: Option, - flag_manifest_path: Option) + flag_manifest_path: Option, flag_features: Vec) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { shell.set_verbose(options.flag_verbose); @@ -36,6 +38,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult jobs: options.flag_jobs, target: options.flag_target.as_ref().map(|t| t.as_slice()), dev_deps: true, + features: options.flag_features.as_slice(), + no_default_features: options.flag_no_default_features, }; let err = try!(ops::run(&root, &mut compile_opts, diff --git a/src/bin/test.rs b/src/bin/test.rs index a74d1ff4c..0355e1c1a 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -16,6 +16,8 @@ Options: -h, --help Print this message --no-run Compile, but don't run tests -j N, --jobs N The number of jobs to run in parallel + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature --target TRIPLE Build for the target triple -u, --update-remotes Deprecated option, use `cargo update` instead --manifest-path PATH Path to the manifest to build tests for @@ -24,7 +26,7 @@ Options: All of the trailing arguments are passed to the test binaries generated for filtering tests and generally providing options configuring how they run. ", flag_jobs: Option, flag_target: Option, - flag_manifest_path: Option) + flag_manifest_path: Option, flag_features: Vec) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path)); @@ -39,6 +41,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult jobs: options.flag_jobs, target: options.flag_target.as_ref().map(|s| s.as_slice()), dev_deps: true, + features: options.flag_features.as_slice(), + no_default_features: options.flag_no_default_features, }, }; diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 0e9add848..fd3e5b961 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -9,10 +9,15 @@ pub struct Dependency { req: VersionReq, transitive: bool, only_match_name: bool, + + optional: bool, + default_features: bool, + features: Vec, } impl Dependency { - pub fn parse(name: &str, version: Option<&str>, + pub fn parse(name: &str, + version: Option<&str>, source_id: &SourceId) -> CargoResult { let version = match version { Some(v) => try!(VersionReq::parse(v)), @@ -20,11 +25,9 @@ impl Dependency { }; Ok(Dependency { - name: name.to_string(), - source_id: source_id.clone(), - req: version, - transitive: true, only_match_name: false, + req: version, + .. Dependency::new_override(name, source_id) }) } @@ -35,6 +38,9 @@ impl Dependency { req: VersionReq::any(), transitive: true, only_match_name: true, + optional: false, + features: Vec::new(), + default_features: true, } } @@ -50,16 +56,31 @@ impl Dependency { &self.source_id } - pub fn as_dev(&self) -> Dependency { - let mut dep = self.clone(); - dep.transitive = false; - dep + pub fn transitive(mut self, transitive: bool) -> Dependency { + self.transitive = transitive; + self } - pub fn is_transitive(&self) -> bool { - self.transitive + pub fn features(mut self, features: Vec) -> Dependency { + self.features = features; + self } + pub fn default_features(mut self, default_features: bool) -> Dependency { + self.default_features = default_features; + self + } + + pub fn optional(mut self, optional: bool) -> Dependency { + self.optional = optional; + self + } + + pub fn is_transitive(&self) -> bool { self.transitive } + pub fn is_optional(&self) -> bool { self.optional } + pub fn uses_default_features(&self) -> bool { self.default_features } + pub fn get_features(&self) -> &[String] { self.features.as_slice() } + pub fn matches(&self, sum: &Summary) -> bool { debug!("matches; self={}; summary={}", self, sum); debug!(" a={}; b={}", self.source_id, sum.get_source_id()); diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index b3e8ab052..fced063ef 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -1,7 +1,9 @@ use std::hash; use std::fmt::{mod, Show, Formatter}; + use semver::Version; use serialize::{Encoder,Encodable}; + use core::source::SourceId; use core::{ Dependency, @@ -333,15 +335,15 @@ impl Show for Target { impl Manifest { - pub fn new(summary: &Summary, targets: &[Target], - target_dir: &Path, doc_dir: &Path, sources: Vec, + pub fn new(summary: Summary, targets: Vec, + target_dir: Path, doc_dir: Path, sources: Vec, build: Vec, exclude: Vec) -> Manifest { Manifest { - summary: summary.clone(), + summary: summary, authors: Vec::new(), - targets: targets.to_vec(), - target_dir: target_dir.clone(), - doc_dir: doc_dir.clone(), + targets: targets, + target_dir: target_dir, + doc_dir: doc_dir, sources: sources, build: build, warnings: Vec::new(), diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs index 6815fcc78..efc9adb89 100644 --- a/src/cargo/core/resolver.rs +++ b/src/cargo/core/resolver.rs @@ -1,20 +1,29 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt; +use semver; use serialize::{Encodable, Encoder, Decodable, Decoder}; -use semver; -use core::{Dependency, PackageId, Registry, SourceId, PackageIdSpec}; -use util::graph::{Nodes, Edges}; -use util::profile; +use core::{PackageId, Registry, SourceId, Summary, Dependency}; +use core::PackageIdSpec; use util::{CargoResult, Graph, human, internal, ChainError}; +use util::profile; +use util::graph::{Nodes, Edges}; #[deriving(PartialEq, Eq)] pub struct Resolve { graph: Graph, + features: HashMap>, root: PackageId } +pub enum ResolveMethod<'a> { + ResolveEverything, + ResolveRequired(/* dev_deps = */ bool, + /* features = */ &'a [String], + /* uses_default_features = */ bool), +} + #[deriving(Encodable, Decodable, Show)] pub struct EncodableResolve { package: Option>, @@ -37,7 +46,7 @@ impl EncodableResolve { } let root = self.root.to_package_id(default); - Ok(Resolve { graph: g, root: try!(root) }) + Ok(Resolve { graph: g, root: try!(root), features: HashMap::new() }) } } @@ -189,7 +198,7 @@ impl Resolve { fn new(root: PackageId) -> Resolve { let mut g = Graph::new(); g.add(root.clone(), []); - Resolve { graph: g, root: root } + Resolve { graph: g, root: root, features: HashMap::new() } } pub fn iter(&self) -> Nodes { @@ -225,6 +234,10 @@ impl Resolve { None => Ok(ret) } } + + pub fn features(&self, pkg: &PackageId) -> Option<&HashSet> { + self.features.find(pkg) + } } impl fmt::Show for Resolve { @@ -236,11 +249,16 @@ impl fmt::Show for Resolve { struct Context<'a, R:'a> { registry: &'a mut R, resolve: Resolve, + // cycle detection + visited: HashSet, + + // Try not to re-resolve too much + resolved: HashMap>, // Eventually, we will have smarter logic for checking for conflicts in the // resolve, but without the registry, conflicts should not exist in // practice, so this is just a sanity check. - seen: HashMap<(String, SourceId), semver::Version> + seen: HashMap<(String, SourceId), semver::Version>, } impl<'a, R: Registry> Context<'a, R> { @@ -248,44 +266,87 @@ impl<'a, R: Registry> Context<'a, R> { Context { registry: registry, resolve: Resolve::new(root), - seen: HashMap::new() + seen: HashMap::new(), + visited: HashSet::new(), + resolved: HashMap::new(), } } } -pub fn resolve(root: &PackageId, deps: &[Dependency], +pub fn resolve(summary: &Summary, method: ResolveMethod, registry: &mut R) -> CargoResult { - log!(5, "resolve; deps={}", deps); - let _p = profile::start(format!("resolving: {}", root)); + log!(5, "resolve; summary={}", summary); + let _p = profile::start(format!("resolving: {}", summary)); - let mut context = Context::new(registry, root.clone()); - try!(resolve_deps(root, deps, &mut context)); + let mut context = Context::new(registry, summary.get_package_id().clone()); + try!(resolve_deps(summary, method, &mut context)); log!(5, " result={}", context.resolve); Ok(context.resolve) } -fn resolve_deps<'a, R: Registry>(parent: &PackageId, - deps: &[Dependency], +fn resolve_deps<'a, R: Registry>(parent: &Summary, + method: ResolveMethod, ctx: &mut Context<'a, R>) -> CargoResult<()> { - if deps.is_empty() { - return Ok(()); + // Dependency graphs are required to be a DAG + if !ctx.visited.insert(parent.get_package_id().clone()) { + return Err(human(format!("Cyclic package dependency: package `{}` \ + depends on itself", parent.get_package_id()))) } - for dep in deps.iter() { - let pkgs = try!(ctx.registry.query(dep)); + let dev_deps = match method { + ResolveEverything => true, + ResolveRequired(dev_deps, _, _) => dev_deps, + }; - if pkgs.is_empty() { - return Err(human(format!("No package named `{:s}` found (required by `{:s}`).\n\ - Location searched: {}\n\ - Version required: {}", - dep.get_name(), - parent.get_name(), - dep.get_source_id(), - dep.get_version_req()))); + // First, filter by dev-dependencies + let deps = parent.get_dependencies(); + let deps = deps.iter().filter(|d| d.is_transitive() || dev_deps); + + // Second, weed out optional dependencies, but not those required + let (mut feature_deps, used_features) = try!(build_features(parent, method)); + let deps = deps.filter(|d| { + !d.is_optional() || feature_deps.remove(&d.get_name().to_string()) + }).collect::>(); + + // All features can only point to optional dependencies, in which case they + // should have all been weeded out by the above iteration. Any remaining + // features are bugs in that the package does not actually have those + // features. + if feature_deps.len() > 0 { + let features = feature_deps.iter().map(|s| s.as_slice()) + .collect::>().connect(", "); + return Err(human(format!("Package `{}` does not have these features: \ + `{}`", parent.get_package_id(), features))) + } + + // Record what list of features is active for this package. + { + let pkgid = parent.get_package_id().clone(); + let features = ctx.resolve.features.find_or_insert(pkgid, + HashSet::new()); + features.extend(used_features.into_iter()); + } + + // Recursively resolve all dependencies + for &dep in deps.iter() { + if !ctx.resolved.find_or_insert(parent.get_package_id().clone(), + HashSet::new()) + .insert(dep.get_name().to_string()) { + continue } - if pkgs.len() > 1 { + let pkgs = try!(ctx.registry.query(dep)); + if pkgs.is_empty() { + return Err(human(format!("No package named `{}` found \ + (required by `{}`).\n\ + Location searched: {}\n\ + Version required: {}", + dep.get_name(), + parent.get_package_id().get_name(), + dep.get_source_id(), + dep.get_version_req()))); + } else if pkgs.len() > 1 { return Err(internal(format!("At the moment, Cargo only supports a \ single source for a particular package name ({}).", dep))); } @@ -293,52 +354,106 @@ fn resolve_deps<'a, R: Registry>(parent: &PackageId, let summary = &pkgs[0]; let name = summary.get_name().to_string(); let source_id = summary.get_source_id().clone(); - let version = summary.get_version().clone(); - - ctx.resolve.graph.link(parent.clone(), summary.get_package_id().clone()); - - let found = { - let found = ctx.seen.find(&(name.clone(), source_id.clone())); - - if found.is_some() { - if found == Some(&version) { continue; } - return Err(human(format!("Cargo found multiple copies of {} in {}. This \ - is not currently supported", - summary.get_name(), summary.get_source_id()))); - } else { - false + let version = summary.get_version(); + + ctx.resolve.graph.link(parent.get_package_id().clone(), + summary.get_package_id().clone()); + + let found = match ctx.seen.find(&(name.clone(), source_id.clone())) { + Some(v) if v == version => true, + Some(..) => { + return Err(human(format!("Cargo found multiple copies of {} in \ + {}. This is not currently supported", + summary.get_name(), + summary.get_source_id()))); } + None => false, }; - if !found { - ctx.seen.insert((name, source_id), version); + ctx.seen.insert((name, source_id), version.clone()); + ctx.resolve.graph.add(summary.get_package_id().clone(), []); } + try!(resolve_deps(summary, + ResolveRequired(false, dep.get_features(), + dep.uses_default_features()), + ctx)); + } - ctx.resolve.graph.add(summary.get_package_id().clone(), []); - - let deps: Vec = summary.get_dependencies().iter() - .filter(|d| d.is_transitive()) - .map(|d| d.clone()) - .collect(); + ctx.visited.remove(parent.get_package_id()); + Ok(()) +} - try!(resolve_deps(summary.get_package_id(), deps.as_slice(), ctx)); +// Returns a pair of (feature dependencies, all used features) +fn build_features(s: &Summary, method: ResolveMethod) + -> CargoResult<(HashSet, HashSet)> { + let mut deps = HashSet::new(); + let mut used = HashSet::new(); + let mut visited = HashSet::new(); + match method { + ResolveEverything => { + for key in s.get_features().keys() { + try!(add_feature(s, key.as_slice(), &mut deps, &mut used, + &mut visited)); + } + } + ResolveRequired(_, requested_features, _) => { + for feat in requested_features.iter() { + try!(add_feature(s, feat.as_slice(), &mut deps, &mut used, + &mut visited)); + } + } } + match method { + ResolveEverything | ResolveRequired(_, _, true) => { + if s.get_features().find_equiv(&"default").is_some() && + !visited.contains_equiv(&"default") { + try!(add_feature(s, "default", &mut deps, &mut used, + &mut visited)); + } + } + _ => {} + } + return Ok((deps, used)); - Ok(()) + fn add_feature(s: &Summary, feat: &str, + deps: &mut HashSet, + used: &mut HashSet, + visited: &mut HashSet) -> CargoResult<()> { + if !visited.insert(feat.to_string()) { + return Err(human(format!("Cyclic feature dependency: feature `{}` \ + depends on itself", feat))) + } + used.insert(feat.to_string()); + match s.get_features().find_equiv(&feat) { + Some(recursive) => { + for f in recursive.iter() { + try!(add_feature(s, f.as_slice(), deps, used, visited)); + } + } + None => { deps.insert(feat.to_string()); } + } + visited.remove(&feat.to_string()); + Ok(()) + } } #[cfg(test)] mod test { + use std::collections::HashMap; + use hamcrest::{assert_that, equal_to, contains}; use core::source::{SourceId, RegistryKind, GitKind}; use core::{Dependency, PackageId, Summary, Registry}; use util::{CargoResult, ToUrl}; - fn resolve(pkg: &PackageId, deps: &[Dependency], + fn resolve(pkg: PackageId, deps: Vec, registry: &mut R) -> CargoResult> { - Ok(try!(super::resolve(pkg, deps, registry)).iter().map(|p| p.clone()).collect()) + let summary = Summary::new(pkg, deps, HashMap::new()).unwrap(); + let method = super::ResolveEverything; + Ok(try!(super::resolve(&summary, method, + registry)).iter().map(|p| p.clone()).collect()) } trait ToDep { @@ -363,13 +478,13 @@ mod test { ($name:expr => $($deps:expr),+) => ({ let d: Vec = vec!($($deps.to_dep()),+); - Summary::new(&PackageId::new($name, "1.0.0", ®istry_loc()).unwrap(), - d.as_slice()) + Summary::new(PackageId::new($name, "1.0.0", ®istry_loc()).unwrap(), + d, HashMap::new()).unwrap() }); ($name:expr) => ( - Summary::new(&PackageId::new($name, "1.0.0", ®istry_loc()).unwrap(), - []) + Summary::new(PackageId::new($name, "1.0.0", ®istry_loc()).unwrap(), + Vec::new(), HashMap::new()).unwrap() ) ) @@ -379,7 +494,7 @@ mod test { } fn pkg(name: &str) -> Summary { - Summary::new(&pkg_id(name), &[]) + Summary::new(pkg_id(name), Vec::new(), HashMap::new()).unwrap() } fn pkg_id(name: &str) -> PackageId { @@ -395,7 +510,7 @@ mod test { } fn pkg_loc(name: &str, loc: &str) -> Summary { - Summary::new(&pkg_id_loc(name, loc), &[]) + Summary::new(pkg_id_loc(name, loc), Vec::new(), HashMap::new()).unwrap() } fn dep(name: &str) -> Dependency { @@ -427,7 +542,8 @@ mod test { #[test] pub fn test_resolving_empty_dependency_list() { - let res = resolve(&pkg_id("root"), [], &mut registry(vec!())).unwrap(); + let res = resolve(pkg_id("root"), Vec::new(), + &mut registry(vec!())).unwrap(); assert_that(&res, equal_to(&names(["root"]))); } @@ -435,7 +551,7 @@ mod test { #[test] pub fn test_resolving_only_package() { let mut reg = registry(vec!(pkg("foo"))); - let res = resolve(&pkg_id("root"), [dep("foo")], &mut reg); + let res = resolve(pkg_id("root"), vec![dep("foo")], &mut reg); assert_that(&res.unwrap(), contains(names(["root", "foo"])).exactly()); } @@ -443,7 +559,7 @@ mod test { #[test] pub fn test_resolving_one_dep() { let mut reg = registry(vec!(pkg("foo"), pkg("bar"))); - let res = resolve(&pkg_id("root"), [dep("foo")], &mut reg); + let res = resolve(pkg_id("root"), vec![dep("foo")], &mut reg); assert_that(&res.unwrap(), contains(names(["root", "foo"])).exactly()); } @@ -451,7 +567,8 @@ mod test { #[test] pub fn test_resolving_multiple_deps() { let mut reg = registry(vec!(pkg!("foo"), pkg!("bar"), pkg!("baz"))); - let res = resolve(&pkg_id("root"), [dep("foo"), dep("baz")], &mut reg).unwrap(); + let res = resolve(pkg_id("root"), vec![dep("foo"), dep("baz")], + &mut reg).unwrap(); assert_that(&res, contains(names(["root", "foo", "baz"])).exactly()); } @@ -459,7 +576,7 @@ mod test { #[test] pub fn test_resolving_transitive_deps() { let mut reg = registry(vec!(pkg!("foo"), pkg!("bar" => "foo"))); - let res = resolve(&pkg_id("root"), [dep("bar")], &mut reg).unwrap(); + let res = resolve(pkg_id("root"), vec![dep("bar")], &mut reg).unwrap(); assert_that(&res, contains(names(["root", "foo", "bar"]))); } @@ -467,7 +584,8 @@ mod test { #[test] pub fn test_resolving_common_transitive_deps() { let mut reg = registry(vec!(pkg!("foo" => "bar"), pkg!("bar"))); - let res = resolve(&pkg_id("root"), [dep("foo"), dep("bar")], &mut reg).unwrap(); + let res = resolve(pkg_id("root"), vec![dep("foo"), dep("bar")], + &mut reg).unwrap(); assert_that(&res, contains(names(["root", "foo", "bar"]))); } @@ -475,16 +593,16 @@ mod test { #[test] pub fn test_resolving_with_same_name() { let list = vec![pkg_loc("foo", "http://first.example.com"), - pkg_loc("foo", "http://second.example.com")]; + pkg_loc("bar", "http://second.example.com")]; let mut reg = registry(list); - let res = resolve(&pkg_id("root"), - [dep_loc("foo", "http://first.example.com"), - dep_loc("foo", "http://second.example.com")], - &mut reg); + let res = resolve(pkg_id("root"), + vec![dep_loc("foo", "http://first.example.com"), + dep_loc("bar", "http://second.example.com")], + &mut reg); let mut names = loc_names([("foo", "http://first.example.com"), - ("foo", "http://second.example.com")]); + ("bar", "http://second.example.com")]); names.push(pkg_id("root")); @@ -494,13 +612,15 @@ mod test { #[test] pub fn test_resolving_with_dev_deps() { let mut reg = registry(vec!( - pkg!("foo" => "bar", dep("baz").as_dev()), - pkg!("baz" => "bat", dep("bam").as_dev()), + pkg!("foo" => "bar", dep("baz").transitive(false)), + pkg!("baz" => "bat", dep("bam").transitive(false)), pkg!("bar"), pkg!("bat") )); - let res = resolve(&pkg_id("root"), [dep("foo"), dep("baz").as_dev()], &mut reg).unwrap(); + let res = resolve(pkg_id("root"), + vec![dep("foo"), dep("baz").transitive(false)], + &mut reg).unwrap(); assert_that(&res, contains(names(["root", "foo", "bar", "baz"]))); } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index c3f1bb1f1..c69fc9500 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -1,23 +1,62 @@ +use std::collections::HashMap; + use semver::Version; -use core::{ - Dependency, - PackageId, - SourceId -}; +use core::{Dependency, PackageId, SourceId}; + +use util::{CargoResult, human}; /// Summaries are cloned, and should not be mutated after creation #[deriving(Show,Clone,PartialEq)] pub struct Summary { package_id: PackageId, - dependencies: Vec + dependencies: Vec, + features: HashMap>, } impl Summary { - pub fn new(pkg_id: &PackageId, dependencies: &[Dependency]) -> Summary { - Summary { - package_id: pkg_id.clone(), - dependencies: dependencies.to_vec(), + pub fn new(pkg_id: PackageId, + dependencies: Vec, + features: HashMap>) -> CargoResult { + for dep in dependencies.iter() { + if features.find_equiv(&dep.get_name()).is_some() { + return Err(human(format!("Features and dependencies cannot have \ + the same name: `{}`", dep.get_name()))) + } + if dep.is_optional() && !dep.is_transitive() { + return Err(human(format!("Dev-dependencies are not allowed \ + to be optional: `{}`", + dep.get_name()))) + } + } + for (feature, list) in features.iter() { + for dep in list.iter() { + if features.find_equiv(&dep.as_slice()).is_some() { continue } + let d = dependencies.iter().find(|d| { + d.get_name() == dep.as_slice() + }); + match d { + Some(d) => { + if d.is_optional() { continue } + return Err(human(format!("Feature `{}` depends on `{}` \ + which is not an optional \ + dependency.\nConsider adding \ + `optional = true` to the \ + dependency", feature, dep))) + } + None => { + return Err(human(format!("Feature `{}` includes `{}` \ + which is neither a dependency \ + nor another feature", + feature, dep))) + } + } + } } + Ok(Summary { + package_id: pkg_id, + dependencies: dependencies, + features: features, + }) } pub fn get_package_id(&self) -> &PackageId { @@ -39,6 +78,10 @@ impl Summary { pub fn get_dependencies(&self) -> &[Dependency] { self.dependencies.as_slice() } + + pub fn get_features(&self) -> &HashMap> { + &self.features + } } pub trait SummaryVec { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index f9cce0b71..b569e2c50 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -40,14 +40,19 @@ pub struct CompileOptions<'a> { pub jobs: Option, pub target: Option<&'a str>, pub dev_deps: bool, + pub features: &'a [String], + pub no_default_features: bool, } pub fn compile(manifest_path: &Path, options: &mut CompileOptions) -> CargoResult { let CompileOptions { update, env, ref mut shell, jobs, target, - dev_deps } = *options; + dev_deps, features, no_default_features } = *options; let target = target.map(|s| s.to_string()); + let features = features.iter().flat_map(|s| { + s.as_slice().split(' ') + }).map(|s| s.to_string()).collect::>(); log!(4, "compile; manifest-path={}", manifest_path.display()); @@ -84,13 +89,11 @@ pub fn compile(manifest_path: &Path, // overrides, etc. let _p = profile::start("resolving w/ overrides..."); - let dependencies = package.get_dependencies().iter().filter(|dep| { - dep.is_transitive() || dev_deps - }).map(|d| d.clone()).collect::>(); try!(registry.add_overrides(override_ids)); + let method = resolver::ResolveRequired(dev_deps, features.as_slice(), + !no_default_features); let resolved_with_overrides = - try!(resolver::resolve(package.get_package_id(), - dependencies.as_slice(), + try!(resolver::resolve(package.get_summary(), method, &mut registry)); let req: Vec = resolved_with_overrides.iter().map(|r| { diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs index 4120c4384..310b18ca5 100644 --- a/src/cargo/ops/cargo_fetch.rs +++ b/src/cargo/ops/cargo_fetch.rs @@ -32,8 +32,8 @@ pub fn resolve_and_fetch(registry: &mut PackageRegistry, package: &Package) None => try!(registry.add_sources(package.get_source_ids())), } - let resolved = try!(resolver::resolve(package.get_package_id(), - package.get_dependencies(), + let resolved = try!(resolver::resolve(package.get_summary(), + resolver::ResolveEverything, registry)); try!(ops::write_resolve(package, &resolved)); Ok(resolved) @@ -58,7 +58,7 @@ fn add_lockfile_sources(registry: &mut PackageRegistry, resolve: &Resolve) -> CargoResult<()> { let deps = resolve.deps(root.get_package_id()).into_iter().flat_map(|deps| { deps.map(|d| (d.get_name(), d)) - }).collect::>(); + }).collect::>(); let mut sources = vec![root.get_package_id().get_source_id().clone()]; let mut to_avoid = HashSet::new(); @@ -66,7 +66,8 @@ fn add_lockfile_sources(registry: &mut PackageRegistry, for dep in root.get_dependencies().iter() { match deps.find(&dep.get_name()) { Some(&lockfile_dep) => { - let summary = Summary::new(lockfile_dep, []); + let summary = Summary::new(lockfile_dep.clone(), Vec::new(), + HashMap::new()).unwrap(); if dep.matches(&summary) { fill_with_deps(resolve, lockfile_dep, &mut to_add); } else { diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 05dac10b3..b2132e047 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -24,8 +24,8 @@ pub fn generate_lockfile(manifest_path: &Path, let resolve = { let mut registry = PackageRegistry::new(&mut config); try!(registry.add_sources(source_ids)); - try!(resolver::resolve(package.get_package_id(), - package.get_dependencies(), + try!(resolver::resolve(package.get_summary(), + resolver::ResolveEverything, &mut registry)) }; try!(write_resolve(&package, &resolve)); @@ -66,8 +66,8 @@ pub fn update_lockfile(manifest_path: &Path, }; try!(registry.add_sources(sources)); - let resolve = try!(resolver::resolve(package.get_package_id(), - package.get_dependencies(), + let resolve = try!(resolver::resolve(package.get_summary(), + resolver::ResolveEverything, &mut registry)); try!(write_resolve(&package, &resolve)); diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 228dae234..37b146653 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -69,12 +69,15 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, let are_files_fresh = use_pkg || try!(calculate_target_fresh(pkg, &old_dep_info)); - // Second bit of the freshness calculation, whether rustc itself and the - // target are fresh. + // Second bit of the freshness calculation, whether rustc itself, the + // target are fresh, and the enabled set of features are all fresh. + let features = cx.resolve.features(pkg.get_package_id()); + let features = features.map(|s| s.iter().collect::>()); let rustc_fingerprint = if use_pkg { - mk_fingerprint(cx, &(target, try!(calculate_pkg_fingerprint(cx, pkg)))) + mk_fingerprint(cx, &(target, try!(calculate_pkg_fingerprint(cx, pkg)), + features)) } else { - mk_fingerprint(cx, target) + mk_fingerprint(cx, &(target, features)) }; let is_rustc_fresh = try!(is_fresh(&old_loc, rustc_fingerprint.as_slice())); diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index ea59bab11..4a7553d3f 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -259,7 +259,7 @@ fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, cx: &Context, req: PlatformRequirement) -> CargoResult> { let base = process("rustc", package, cx); - let base = build_base_args(cx, base, target, crate_types.as_slice()); + let base = build_base_args(cx, base, package, target, crate_types.as_slice()); let target_cmd = build_plugin_args(base.clone(), cx, package, target, KindTarget); let plugin_cmd = build_plugin_args(base, cx, package, target, KindPlugin); @@ -317,7 +317,9 @@ fn rustdoc(package: &Package, target: &Target, }, desc)) } -fn build_base_args(cx: &Context, mut cmd: ProcessBuilder, +fn build_base_args(cx: &Context, + mut cmd: ProcessBuilder, + pkg: &Package, target: &Target, crate_types: &[&str]) -> ProcessBuilder { let metadata = target.get_metadata(); @@ -361,6 +363,15 @@ fn build_base_args(cx: &Context, mut cmd: ProcessBuilder, cmd = cmd.arg("--test"); } + match cx.resolve.features(pkg.get_package_id()) { + Some(features) => { + for feat in features.iter() { + cmd = cmd.arg("--cfg").arg(format!("feature=\"{}\"", feat)); + } + } + None => {} + } + match metadata { Some(m) => { cmd = cmd.arg("-C").arg(format!("metadata={}", m.metadata)); diff --git a/src/cargo/ops/cargo_upload.rs b/src/cargo/ops/cargo_upload.rs index c6df621bc..d1bd7fbd4 100644 --- a/src/cargo/ops/cargo_upload.rs +++ b/src/cargo/ops/cargo_upload.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::io::File; use std::os; use std::str; -use serialize::json; +use serialize::json::{mod, ToJson}; use curl::http; use git2; @@ -84,12 +84,21 @@ fn transmit(pkg: &Package, mut tarball: File, from {} instead", dep.get_name(), dep.get_source_id()))) } - let header = format!("{}|{}", dep.get_name(), dep.get_version_req()); + + // See Registry::parse_registry_dependency for format + let opt = if dep.is_optional() {"-"} else {""}; + let default = if dep.uses_default_features() {""} else {"*"}; + let features = dep.get_features().connect(","); + let header = format!("{}{}{}|{}|{}", opt, default, dep.get_name(), + features, dep.get_version_req()); if i > 0 { dep_header.push_str(";"); } dep_header.push_str(header.as_slice()); } req = req.header("X-Cargo-Pkg-Dep", dep_header.as_slice()); + let feature_header = pkg.get_summary().get_features().to_json().to_string(); + req = req.header("X-Cargo-Pkg-Feature", feature_header.as_slice()); + let response = try!(req.exec()); if response.get_code() == 0 { return Ok(()) } // file upload url diff --git a/src/cargo/sources/registry.rs b/src/cargo/sources/registry.rs index 3a91e78d1..9ce586d4b 100644 --- a/src/cargo/sources/registry.rs +++ b/src/cargo/sources/registry.rs @@ -1,11 +1,9 @@ -#![allow(unused)] use std::io::{mod, fs, File, MemReader}; use std::io::fs::PathExtensions; use std::collections::HashMap; use curl::http; use git2; -use semver::Version; use flate2::reader::GzDecoder; use serialize::json; use serialize::hex::ToHex; @@ -43,6 +41,7 @@ struct RegistryPackage { name: String, vers: String, deps: Vec, + features: HashMap>, cksum: String, } @@ -97,7 +96,6 @@ impl<'a, 'b> RegistrySource<'a, 'b> { try!(fs::mkdir_recursive(&self.checkout_path, io::UserDir)); let _ = fs::rmdir_recursive(&self.checkout_path); - let url = self.source_id.get_url().to_string(); let repo = try!(git2::Repository::init(&self.checkout_path)); Ok(repo) } @@ -177,6 +175,62 @@ impl<'a, 'b> RegistrySource<'a, 'b> { try!(File::create(&dst.join(".cargo-ok"))); Ok(dst) } + + /// Parse a line from the registry's index file into a Summary for a + /// package. + fn parse_registry_package(&mut self, line: &str) -> CargoResult { + let pkg = try!(json::decode::(line)); + let pkgid = try!(PackageId::new(pkg.name.as_slice(), + pkg.vers.as_slice(), + &self.source_id)); + let deps: CargoResult> = pkg.deps.iter().map(|dep| { + self.parse_registry_dependency(dep.as_slice()) + }).collect(); + let deps = try!(deps); + let RegistryPackage { name, vers, cksum, .. } = pkg; + self.hashes.insert((name, vers), cksum); + Summary::new(pkgid, deps, pkg.features) + } + + /// Parse a dependency listed in the registry into a `Dependency`. + /// + /// Currently the format for dependencies is: + /// + /// ```notrust + /// dep := ['-'] ['*'] name '|' [ name ',' ] * '|' version_req + /// ``` + /// + /// The '-' indicates that this is an optional dependency, and the '*' + /// indicates that the dependency does *not* use the default features + /// provided. The comma-separate list of names in brackets are the enabled + /// features for the dependency, and the final element is the version + /// requirement of the dependency. + fn parse_registry_dependency(&self, dep: &str) -> CargoResult { + let mut parts = dep.as_slice().splitn(2, '|'); + let name = parts.next().unwrap(); + let features = try!(parts.next().require(|| { + human(format!("malformed dependency in registry: {}", dep)) + })); + let vers = try!(parts.next().require(|| { + human(format!("malformed dependency in registry: {}", dep)) + })); + let (name, optional) = if name.starts_with("-") { + (name.slice_from(1), true) + } else { + (name, false) + }; + let (name, default_features) = if name.starts_with("*") { + (name.slice_from(1), false) + } else { + (name, true) + }; + let features = features.split(',').filter(|s| !s.is_empty()) + .map(|s| s.to_string()).collect(); + let dep = try!(Dependency::parse(name, Some(vers), &self.source_id)); + Ok(dep.optional(optional) + .default_features(default_features) + .features(features)) + } } impl<'a, 'b> Registry for RegistrySource<'a, 'b> { @@ -198,25 +252,8 @@ impl<'a, 'b> Registry for RegistrySource<'a, 'b> { let ret: CargoResult>; ret = contents.as_slice().lines().filter(|l| l.trim().len() > 0) - .map(|l| { - - let pkg = try!(json::decode::(l)); - let pkgid = try!(PackageId::new(pkg.name.as_slice(), - pkg.vers.as_slice(), - &self.source_id)); - let deps: CargoResult> = pkg.deps.iter().map(|dep| { - let mut parts = dep.as_slice().splitn(1, '|'); - let name = parts.next().unwrap(); - let vers = try!(parts.next().require(|| { - human(format!("malformed dependency in registry: {}", dep)) - })); - Dependency::parse(name, Some(vers), &self.source_id) - }).collect(); - let deps = try!(deps); - let RegistryPackage { name, vers, cksum, .. } = pkg; - self.hashes.insert((name, vers), cksum); - Ok(Summary::new(&pkgid, deps.as_slice())) - }).collect(); + .map(|l| self.parse_registry_package(l)) + .collect(); let mut summaries = try!(ret.chain_error(|| { internal(format!("Failed to parse registry's information for: {}", dep.get_name())) diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 1e417f549..37430b2a1 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -185,14 +185,17 @@ pub enum TomlDependency { } -#[deriving(Decodable)] +#[deriving(Decodable, Clone, Default)] pub struct DetailedTomlDependency { version: Option, path: Option, git: Option, branch: Option, tag: Option, - rev: Option + rev: Option, + features: Option>, + optional: Option, + default_features: Option, } #[deriving(Decodable)] @@ -207,6 +210,7 @@ pub struct TomlManifest { bench: Option>, dependencies: Option>, dev_dependencies: Option>, + features: Option>>, } #[deriving(Decodable, Clone, Default)] @@ -464,11 +468,13 @@ impl TomlManifest { }; let exclude = project.exclude.clone().unwrap_or(Vec::new()); - let summary = Summary::new(&pkgid, deps.as_slice()); - let mut manifest = Manifest::new(&summary, - targets.as_slice(), - &layout.root.join("target"), - &layout.root.join("doc"), + let summary = try!(Summary::new(pkgid, deps, + self.features.clone() + .unwrap_or(HashMap::new()))); + let mut manifest = Manifest::new(summary, + targets, + layout.root.join("target"), + layout.root.join("doc"), sources, build, exclude); @@ -488,46 +494,47 @@ fn process_dependencies<'a>(cx: &mut Context<'a>, dev: bool, None => return Ok(()) }; for (n, v) in dependencies.iter() { - let (version, source_id) = match *v { - SimpleDep(ref string) => { - (Some(string.clone()), try!(SourceId::for_central())) - }, - DetailedDep(ref details) => { - let reference = details.branch.clone() - .or_else(|| details.tag.clone()) - .or_else(|| details.rev.clone()) - .unwrap_or_else(|| "master".to_string()); - - let new_source_id = match details.git { - Some(ref git) => { - let kind = GitKind(reference.clone()); - let loc = try!(git.as_slice().to_url().map_err(|e| { - human(e) - })); - let source_id = SourceId::new(kind, loc); - // TODO: Don't do this for path - cx.source_ids.push(source_id.clone()); - Some(source_id) - } - None => { - details.path.as_ref().map(|path| { - cx.nested_paths.push(Path::new(path.as_slice())); - cx.source_id.clone() - }) - } - }.unwrap_or(try!(SourceId::for_central())); - - (details.version.clone(), new_source_id) + let details = match *v { + SimpleDep(ref version) => { + let mut d: DetailedTomlDependency = Default::default(); + d.version = Some(version.clone()); + d } + DetailedDep(ref details) => details.clone(), }; - - let mut dep = try!(Dependency::parse(n.as_slice(), - version.as_ref().map(|v| v.as_slice()), - &source_id)); - - if dev { dep = dep.as_dev() } - - cx.deps.push(dep) + let reference = details.branch.clone() + .or_else(|| details.tag.clone()) + .or_else(|| details.rev.clone()) + .unwrap_or_else(|| "master".to_string()); + + let new_source_id = match details.git { + Some(ref git) => { + let kind = GitKind(reference.clone()); + let loc = try!(git.as_slice().to_url().map_err(|e| { + human(e) + })); + let source_id = SourceId::new(kind, loc); + // TODO: Don't do this for path + cx.source_ids.push(source_id.clone()); + Some(source_id) + } + None => { + details.path.as_ref().map(|path| { + cx.nested_paths.push(Path::new(path.as_slice())); + cx.source_id.clone() + }) + } + }.unwrap_or(try!(SourceId::for_central())); + + let dep = try!(Dependency::parse(n.as_slice(), + details.version.as_ref() + .map(|v| v.as_slice()), + &new_source_id)); + let dep = dep.transitive(!dev) + .features(details.features.unwrap_or(Vec::new())) + .default_features(details.default_features.unwrap_or(true)) + .optional(details.optional.unwrap_or(false)); + cx.deps.push(dep); } Ok(()) diff --git a/src/doc/manifest.md b/src/doc/manifest.md index 87a9aeb83..587654cf0 100644 --- a/src/doc/manifest.md +++ b/src/doc/manifest.md @@ -130,6 +130,137 @@ opt-level = 0 debug = true ``` +# The `[features]` Section + +Cargo supports **features** to allow expression of: + +* Optional dependencies, which enhance a package, but are not required +* Clusters of optional dependencies, such as "postgres", that would include the + `postgres` package, the `postgres-macros` package, and possibly other packages + (such as development-time mocking libraries, debugging tools, etc.) + +The format for specifying features is: + +```toml +[package] +name = "awesome" + +[features] + +# The "default" set of optional packages. Most people will +# want to use these packages, but they are strictly optional +default = ["jquery", "uglifier"] + +# The "secure-password" feature depends on the bcrypt package. +# This aliasing will allow people to talk about the feature in +# a higher-level way and allow this package to add more +# requirements to the feature in the future. +secure-password = ["bcrypt"] + +[dependencies] + +# These packages are mandatory and form the core of this +# package's distribution +cookie = "1.2.0" +oauth = "1.1.0" +route-recognizer = "=2.1.0" + +# A list of all of the optional dependencies, some of which +# are included in the above "features". They can be opted +# into by apps. +[dependencies.jquery] +version = "1.0.2" +optional = true + +[dependencies.uglifier] +version = "1.5.3" +optional = true + +[dependencies.bcrypt] +version = "*" +optional = true + +[dependencies.civet] +version = "*" +optional = true +``` + +To use the package `awesome`: + +```toml +[dependencies.awesome] +version = "1.3.5" +features = ["secure-password", "civet"] + +# do not include the default features, and optionally +# cherry-pick individual features +default-features = false +``` + +## Rules + +The usage of features is subject to a few rules: + +1. Feature names must not conflict with other package names in the manifest. + This is because they are opted into via `features = [...]`, which only has a + single namespace +2. With the exception of the `default` feature, all features are opt-in. To opt + out of the default feature, use `default-features = false` and cherry-pick + individual features. +3. Feature groups are not allowed to cyclicly depend on one another. +4. Dev-dependencies cannot be optional +5. Features groups can only reference optional dependencies +6. When a feature is selected, Cargo will call `rustc` with + `--cfg feature="${feature_name}"`. If a feature group is included, + it and all of its individual features will be included. This can be + tested in code via `#[cfg(feature = "foo")]` + +Note that it is explicitly allowed for features to not actually activate any +optional dependencies. This allows packages to internally enable/disable +features without requiring a new dependency. + +## Usage In End Products + +One major use-case for this feature is specifying optional features in +end-products. For example, the Servo project may want to include optional +features that people can enable or disable when they build it. + +In that case, Servo will describe features in its `Cargo.toml` and they can be +enabled using command-line flags: + +``` +$ cargo build --release --features "shumway pdf" +``` + +Default features could be excluded using `--no-default-features`. + +## Usage In Packages + +In most cases, the concept of "optional dependency" in a library is best +expressed as a separate package that the top-level application depends on. + +However, high-level packages, like Iron or Piston, may want the ability to +curate a number of packages for easy installation. The current Cargo system +allows them to curate a number of mandatory dependencies into a single package +for easy installation. + +In some cases, packages may want to provide additional curation for **optional** +dependencies: + +* Grouping a number of low-level optional dependencies together into a single + high-level "feature". +* Specifying packages that are recommended (or suggested) to be included by + users of the package. +* Including a feature (like `secure-password` in the motivating example) that + will only work if an optional dependency is available, and would be difficult + to implement as a separate package. For example, it may be overly difficult to + design an IO package to be completely decoupled from OpenSSL, with opt-in via + the inclusion of a separate package. + +In almost all cases, it is an antipattern to use these features outside of +high-level packages that are designed for curation. If a feature is optional, it +can almost certainly be expressed as a separate package. + # The `[dev-dependencies.*]` Sections The format of this section is equivalent to `[dependencies.*]`. Dev-dependencies diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 3d3c569d4..f84260339 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -1082,7 +1082,9 @@ test!(self_dependency { "#) .file("src/test.rs", "fn main() {}"); assert_that(p.cargo_process("build"), - execs().with_status(0)); + execs().with_status(101).with_stderr("\ +Cyclic package dependency: package `test v0.0.0 ([..])` depends on itself +")); }) #[cfg(not(windows))] diff --git a/tests/test_cargo_features.rs b/tests/test_cargo_features.rs new file mode 100644 index 000000000..2f6f4d2ad --- /dev/null +++ b/tests/test_cargo_features.rs @@ -0,0 +1,426 @@ +use support::{project, execs, cargo_dir}; +use support::COMPILING; +use hamcrest::assert_that; + +fn setup() { +} + +test!(invalid1 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + bar = ["baz"] + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr(format!("\ +Cargo.toml is not a valid manifest + +Feature `bar` includes `baz` which is neither a dependency nor another feature +").as_slice())); +}) + +test!(invalid2 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + bar = ["baz"] + + [dependencies.bar] + path = "foo" + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr(format!("\ +Cargo.toml is not a valid manifest + +Features and dependencies cannot have the same name: `bar` +").as_slice())); +}) + +test!(invalid3 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + bar = ["baz"] + + [dependencies.baz] + path = "foo" + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr(format!("\ +Cargo.toml is not a valid manifest + +Feature `bar` depends on `baz` which is not an optional dependency. +Consider adding `optional = true` to the dependency +").as_slice())); +}) + +test!(invalid4 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + features = ["bar"] + "#) + .file("src/main.rs", "") + .file("bar/Cargo.toml", r#" + [project] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr(format!("\ +Package `bar v0.0.1 ([..])` does not have these features: `bar` +").as_slice())); + + let p = p.file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + "#); + + assert_that(p.cargo_process("build").arg("--features").arg("test"), + execs().with_status(101).with_stderr(format!("\ +Package `foo v0.0.1 ([..])` does not have these features: `test` +").as_slice())); +}) + +test!(invalid5 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dev-dependencies.bar] + path = "bar" + optional = true + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr(format!("\ +Cargo.toml is not a valid manifest + +Dev-dependencies are not allowed to be optional: `bar` +").as_slice())); +}) + +test!(no_feature_doesnt_build { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + optional = true + "#) + .file("src/main.rs", r#" + #[cfg(feature = "bar")] + extern crate bar; + #[cfg(feature = "bar")] + fn main() { bar::bar(); println!("bar") } + #[cfg(not(feature = "bar"))] + fn main() {} + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}"); + + assert_that(p.cargo_process("build"), + execs().with_status(0).with_stdout(format!("\ +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); + assert_that(p.process(p.bin("foo")), execs().with_status(0).with_stdout("")); + + assert_that(p.process(cargo_dir().join("cargo")).arg("build") + .arg("--features").arg("bar"), + execs().with_status(0).with_stdout(format!("\ +{compiling} bar v0.0.1 ({dir}) +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); + assert_that(p.process(p.bin("foo")), + execs().with_status(0).with_stdout("bar\n")); +}) + +test!(default_feature_pulled_in { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + default = ["bar"] + + [dependencies.bar] + path = "bar" + optional = true + "#) + .file("src/main.rs", r#" + #[cfg(feature = "bar")] + extern crate bar; + #[cfg(feature = "bar")] + fn main() { bar::bar(); println!("bar") } + #[cfg(not(feature = "bar"))] + fn main() {} + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}"); + + assert_that(p.cargo_process("build"), + execs().with_status(0).with_stdout(format!("\ +{compiling} bar v0.0.1 ({dir}) +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); + assert_that(p.process(p.bin("foo")), + execs().with_status(0).with_stdout("bar\n")); + + assert_that(p.process(cargo_dir().join("cargo")).arg("build") + .arg("--no-default-features"), + execs().with_status(0).with_stdout(format!("\ +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); + assert_that(p.process(p.bin("foo")), execs().with_status(0).with_stdout("")); +}) + +test!(cyclic_feature { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + default = ["default"] + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr("\ +Cyclic feature dependency: feature `default` depends on itself +")); +}) + +test!(cyclic_feature2 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + foo = ["bar"] + bar = ["foo"] + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(101).with_stderr("\ +Cyclic feature dependency: feature `[..]` depends on itself +")); +}) + +test!(groups_on_groups_on_groups { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + default = ["f1"] + f1 = ["f2", "bar"] + f2 = ["f3", "f4"] + f3 = ["f5", "f6", "baz"] + f4 = ["f5", "f7"] + f5 = ["f6"] + f6 = ["f7"] + f7 = ["bar"] + + [dependencies.bar] + path = "bar" + optional = true + + [dependencies.baz] + path = "baz" + optional = true + "#) + .file("src/main.rs", r#" + extern crate bar; + extern crate baz; + fn main() {} + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}") + .file("baz/Cargo.toml", r#" + [package] + name = "baz" + version = "0.0.1" + authors = [] + "#) + .file("baz/src/lib.rs", "pub fn baz() {}"); + + assert_that(p.cargo_process("build"), + execs().with_status(0).with_stdout(format!("\ +{compiling} ba[..] v0.0.1 ({dir}) +{compiling} ba[..] v0.0.1 ({dir}) +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); +}) + +test!(many_cli_features { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + optional = true + + [dependencies.baz] + path = "baz" + optional = true + "#) + .file("src/main.rs", r#" + extern crate bar; + extern crate baz; + fn main() {} + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}") + .file("baz/Cargo.toml", r#" + [package] + name = "baz" + version = "0.0.1" + authors = [] + "#) + .file("baz/src/lib.rs", "pub fn baz() {}"); + + assert_that(p.cargo_process("build").arg("--features").arg("bar baz"), + execs().with_status(0).with_stdout(format!("\ +{compiling} ba[..] v0.0.1 ({dir}) +{compiling} ba[..] v0.0.1 ({dir}) +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); +}) + +test!(union_features { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.d1] + path = "d1" + features = ["f1"] + [dependencies.d2] + path = "d2" + features = ["f2"] + "#) + .file("src/main.rs", r#" + extern crate d1; + extern crate d2; + fn main() { + d2::f1(); + d2::f2(); + } + "#) + .file("d1/Cargo.toml", r#" + [package] + name = "d1" + version = "0.0.1" + authors = [] + + [features] + f1 = ["d2"] + + [dependencies.d2] + path = "../d2" + features = ["f1"] + optional = true + "#) + .file("d1/src/lib.rs", "") + .file("d2/Cargo.toml", r#" + [package] + name = "d2" + version = "0.0.1" + authors = [] + + [features] + f1 = [] + f2 = [] + "#) + .file("d2/src/lib.rs", r#" + #[cfg(feature = "f1")] pub fn f1() {} + #[cfg(feature = "f2")] pub fn f2() {} + "#); + + assert_that(p.cargo_process("build"), + execs().with_status(0).with_stdout(format!("\ +{compiling} d2 v0.0.1 ({dir}) +{compiling} d1 v0.0.1 ({dir}) +{compiling} foo v0.0.1 ({dir}) +", compiling = COMPILING, dir = p.url()).as_slice())); +}) diff --git a/tests/test_cargo_registry.rs b/tests/test_cargo_registry.rs index f47d3a154..37342a573 100644 --- a/tests/test_cargo_registry.rs +++ b/tests/test_cargo_registry.rs @@ -46,7 +46,7 @@ fn setup() { {{"dl":"{}","upload":""}} "#, dl_url()).as_slice()) .file("3/f/foo", pkg("foo", "0.0.1", [], &foo_cksum)) - .file("3/b/bar", pkg("bar", "0.0.1", ["foo|>=0.0.0"], &bar_cksum)) + .file("3/b/bar", pkg("bar", "0.0.1", ["foo||>=0.0.0"], &bar_cksum)) .file("ba/d-/bad-cksum", pkg("bad-cksum", "0.0.1", [], &bar_cksum)) .nocommit_file("no/ty/notyet", pkg("notyet", "0.0.1", [], ¬yet)) .build(); @@ -57,7 +57,7 @@ fn setup() { }).collect(); let deps = deps.connect(","); - format!(r#"{{"name":"{}","vers":"{}","deps":[{}],"cksum":"{}"}}"#, + format!(r#"{{"name":"{}","vers":"{}","deps":[{}],"cksum":"{}","features":{{}}}}"#, name, vers, deps, cksum) } fn dl(path: &str, contents: &[u8]) -> String { diff --git a/tests/tests.rs b/tests/tests.rs index 1c7e95c40..488a4f66f 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -39,6 +39,7 @@ mod test_cargo_version; mod test_cargo_new; mod test_cargo_compile_plugins; mod test_cargo_doc; +mod test_cargo_features; mod test_cargo_freshness; mod test_cargo_generate_lockfile; mod test_cargo_profiles; -- 2.30.2